Hmm, I've got the impression that you are trying to bind to the style property (for example, by doing <div [style]="exp"> which obviously won't work since this property is read-only (as indicated in the error message) and non-simple value.

What you should be doing instead today is one of:

• using a special-case syntax in the compiler, ex.: <div [style.font-color]="exp">
• using the built-in NgStyle directive

That does have a lot of sense.... But I'm not the one trying to do that but something in the e2e tests... Although you sound completely right. This might not be the place to solve it. I'll review it and report back. Thank you!

As said you were totally right... The problem is in modules\benchmarks\src\naive_infinite_scroll\cells.ts
and modules\benchmarks\src\naive_infinite_scroll\scroll_area.ts... I'll try to change'em and update the PR

Oh, I didn't realise it was in the e2e tests... This doesn't look right and I'm surprised that it works at all and passes tests on the CI server :-/

Yes... I was surprised it was just happening to me... Anyway I was trying to solve this when I realized this can be a problem when dealing with components... I mean without the fix in the PR this is supposed to work like you said:

<div [style.font-color]="exp">

But what if instead of a plain div we have a component... In order to use that syntax in it's template it should know in advance which styles it's going to accept (probably all of them). Doesn't it have more sense for the angular framework to abstract components for that complexity? Like:

<my-component [style]="exp">

And then:

@Component({selector: 'my-component'})
@View({
  template: `<div [style]="exp"></div>`
})

Anyway, this may be out of the scope of this PR... I'm going to change both files and revert back the other change and upload.

Thanks!

Remember that there is the ng-style directive to bind to any style. So you can do <div [ng-style]="exp"> already.

Oops. Didn't knew... That's what happens when you answer something on the fly without investigation... Sorry for that.

Any way. Back to coding :-).

@alfonso-presa your changes, as done, have no effect really as the NgStyle directive wasn't added to the @View (wasn't listed in the directives collection). But IMO we shouldn't be using additional directives here as this is an e2e / perf tests, so we should not add more elements than necessary.

I can see 2 solutions:

• use the [style.name]="exp" special-case compiler syntax
• remove binding to styles altogether as it seems that having and not having them doesn't have any effect...

@tbosch do you recall why we play with those styles at all?

My first attempt was to use the `[style.name]="exp"`` but for some reason it broke the dart build (https://travis-ci.org/angular/angular/jobs/69549832).

Dart templates are using NgStyle BTW, so I thought that in shake of consistency it was better to use NgStyle.

About the PR as is... you're right... I forgot to add the directive dependency. Oops.

Anyway I guess is up to you guys to decide which is the best path to solve this before I continue with the PR. IMHO from the developer's perspective the best is to have the [style]="expr" because it's more consistent with the new template syntax and also will work without additional directives.

Thanks!

In the meanwhile I decided to solve the problem as is using NgStyle to fix the tests with the minimum impact in functionality. I guess it can be refactored later if you decide style is not necessary.

BTW.... this now needs #2878 because NgStyle was not exported in directives... Should it be used somehow else?

Thanks!

@alfonso-presa so, we've discussed this one internally, and ideally we would like to avoid using any additional directives in this perf-measuring test. Could you please update your PR by:

• converting from using the NgStyle directive to the [style.font-color]="exp" special-casing in the compiler? So for example this line would become: <div id="scrollDiv" [style.height.px]="viewportHeight" style="width: 1000px; border: 1px solid #000; overflow: scroll" on-scroll="onScroll($event)">? This will need changes in JS as well;
• rebasing on top of master
• squashing all the commits into one

Thnx! Let me know if you face any issues!

Of course I can :-). I'll let you know if a face those weird DART build errors again when changing to [style.name]="exp" syntax.

Thank you.

I'm facing the same DART issues I encountered before while trying to solve this. I guess it's because of the Dart transpiled version of the tests... but I'm not sure how to solve the problem:
https://travis-ci.org/angular/angular/jobs/70809349#L1966

Is it mandatory to use the 'MapWrapper.createFromPairs' for the Dart transpiler to work? If I use that then the [style.prop]="map.entry" syntax does not seem to work...

I'll keep on trying tomorrow anyway.

Thanks!

EDIT: MapWrapper also fails....

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

CLAs look good, thanks!

I've been able to make it work by using an external class syntax... But I don't think it's the best way to write that code:

https://github.com/angular/angular/pull/2874/files#diff-6a123cac4f61bcef66473b5c01dd6dacR83

Any advices?

@alfonso-presa I've left a comment - IMO we can make it simpler by moving "hard-coded" style to a template. It would be totally awesome if you could also:

• make sure that the diff is minimal (I think that there is some difference in handling white-spaces)
• squash all the commits into one

I think that we are close, keep up the good work!

Thank you very much for the tip :-). I'm going to try to fix it first and then I'll squash everything together.

Ok. This should be ready to merge now :-). Let me know if anything more is needed.

Just to let you know, about the problems I encountered during the dart build (https://travis-ci.org/angular/angular/jobs/70809349#L1966), it really feels like there is something in ts2dart or in dart2js that is not working properly... I'm investigating this just for fun :-). Any way, it's completely out of the scope of this PR.

Thanks!

Landed as cd532b0. Thnx @alfonso-presa for bearing with me and iterating on this one, great job!

Thank you!

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}